Draft
Conversation
740422e to
91db70e
Compare
Currently WIP because:
- `$(location)` and `$(rootpath)` make vars aren't working correctly
- This is what we tried to fix with the `PYTHON_COVERAGE_TARGET`
variable, which was ultimately not accepted. See also next point.
- Does not use the new toolchain feature that supports adding
coveragepy directly to the python toolchain
- Added together with the new coveragepy support in
bazelbuild/bazel#15590, and is probably better to use than the
`$(rootpath)` and `pip_install` solution we're showing here
- While I'd love to rewrite the blog post to show this instead, I'd
need a bit more time for that
- Python imports are wonky
- For some reason, the sibling module can only be imported under a
parent `python` module. I confirmed this by looking through
`PYTHON_PATH`
- Seems to be a Bazel regression? Strangely nobody is complaining
about this yet, we should probably raise an issue
- Not tested in remote execution
- No access to an engflow setup to test this right this instant
- Since we're not using a toolchain (and even if we were we're using
`pip_install`), this requires a python on the host, so there's a
decent chance it won't work
- Requires bumping Bazel to a version with bazelbuild/bazel#15590
merged, which currently is only available in prereleases
91db70e to
8ea46d3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently WIP because:
$(location)and$(rootpath)make vars aren't working correctlyPYTHON_COVERAGE_TARGETvariable, which was ultimately not accepted. See also next point.coverage_toolattribute topy_runtime. bazelbuild/bazel#15590, and is probably better to use than the$(rootpath)andpip_installsolution we're showing herepythonmodule. I confirmed this by looking throughPYTHON_PATHpip_install), this requires a python on the host, so there's a decent chance it won't workcoverage_toolattribute topy_runtime. bazelbuild/bazel#15590 merged, which currently is only available in prereleases